Add certificate loading APIs with content-based format detection - #1140

Pull request overview

Adds certificate-loading APIs that mirror wolfSSH_ReadKey_file() by detecting certificate format from content, while also tightening certificate algorithm identification and fixing an SSH public key parsing edge case.

Changes:

• Introduces wolfSSH_ReadCert_{buffer,file}() and wolfSSH_CTX_{UseCert,AddRootCert}_file() (content-sniffed PEM/DER and OpenSSH cert-line support where enabled).
• Refactors internal certificate identification to return the wire x509v3-* algorithm (and reject unmappable certs).
• Fixes DoSshPubKey() handling for SSH public key lines that do not end with a trailing newline; adds API test coverage for new cert APIs and the newline edge case.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated no comments.

    }

    if (destIdx >= WOLFSSH_MAX_PVT_KEYS) {
        ret = WS_CTX_KEY_COUNT_E;
    }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Fenrir Automated Review — PR #1140

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Fenrir Automated Review — PR #1140

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Fenrir Automated Review — PR #1140

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Fenrir Automated Review — PR #1140

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅

Reviewed at c7200ca7. All non-blocking -- the flavor issue from the last round is genuinely fixed, and everything below is either a follow-on in the sibling function or a nit.

Verified first. make check passes with --enable-all and with --enable-all --enable-ossh-certs (10/10 each). api.test passes in an OpenSSH-certs-only build, a build with no certificate support at all, and a build with -DWOLFSSH_NO_ECDSA_SHA2_NISTP256. No new warnings in any of them. Clean under ASan and UBSan, and leaks --atExit reports zero leaks -- so the PEM-reject path that releases its intermediate DER does hold up.

I also confirmed the fix rather than taking the tests' word for it: every decode-failure path out of wolfSSH_ReadCert_buffer() returns flavor == WOLFSSH_CERT_FLAVOR_UNKNOWN, including when the caller passes a non-zero flavor in.

The fix stopped at the buffer function

wolfssh/ssh.h:110 now promises:

Past the argument checks every out parameter is written, flavor as UNKNOWN on failure.

wolfSSH_ReadCert_buffer() honors that. wolfSSH_ReadCert_file() (src/ssh.c:2651) does not: when ReadFileIntoBuffer() fails it returns straight out without writing any out parameter. A missing file, an empty file and a directory are all past the argument checks.

Driving it with sentinel values -- out set to a live pointer, outSz to 0xDEADBEEF, flavor to 0xFF:

                    ret     flavor  out     outSz
missing file        -1019   255     STALE   3735928559
empty file          -1019   255     STALE   3735928559
directory           -1019   255     STALE   3735928559
private key DER      -140     0     NULL    0            <- buffer path, correct
ssh pubkey line     -1016     0     NULL    0            <- correct
good cert               0     1     new     798

A caller who reads the header and frees out on the strength of it gets a stale pointer. test_wolfSSH_ReadCert_file() does exercise the missing-file case, but checks only the return code, which is why it is green.

Fix is a few lines -- clear the five out parameters before calling ReadFileIntoBuffer(), the same way the buffer function does at the top.

Nits

DYNTYPE_CERT is not reachable from a public header. ssh.h:109 tells the caller to free out with it, but it is defined in wolfssh/internal.h. examples/echoserver/echoserver.c:2228 has a comment about exactly this problem ("not using WMALLOC because internal.h is not included for DYNTYPE_*"). Worth naming something the caller can actually see, or saying "free with the heap's free" and leaving the type out.

Raw wolfSSL codes can escape the new reader. A truncated DER comes back as -140 (ASN_PARSE_E) rather than a WS_ code -- IdentifyCert() returns wc_ParseCert()'s value verbatim. This is pre-existing behavior, not something the PR introduces, and the tests accommodate it with AssertIntLT(..., 0). Since the header is documenting a new public entry point, it is worth a sentence saying the return may be a wolfSSL code, or mapping it.

The Ed25519 fixture does not reach the branch it was added for. keys/server-cert-ed25519.{pem,der} are rejected by IdentifyCertKey() with WS_UNIMPLEMENTED_E (-1017) before IdentifyCert()'s new WS_INVALID_ALGO_ID branch is consulted -- the test comment says as much. The branch itself is fine and is genuinely covered: I built with -DWOLFSSH_NO_ECDSA_SHA2_NISTP256 and the #else arm asserting WS_INVALID_ALGO_ID passes. So this is only a note that the fixture and the renewcerts.sh/include.am changes are buying a weaker regression test than they look like they are -- they do lock in that an Ed25519 certificate is not accepted under a plain ssh-ed25519 name, which is worth having.

.gitignore. scp_rekey_*.txt looks unrelated to certificates -- fine either way, just flagging it in case it rode along by accident.